Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix useId hydration error in strict mode #3980

Merged
merged 3 commits into from
Feb 2, 2023
Merged

Fix useId hydration error in strict mode #3980

merged 3 commits into from
Feb 2, 2023

Conversation

devongovett
Copy link
Member

Closes #3637, closes #3637

This fixes a hydration error when using useId with React StrictMode. Due to component render functions being called multiple times on the client but not on the server, the ids would mismatch. Several approaches to fixing this have been proposed (thanks to @ciffelia and others). One in #3637 is to switch to React.useId on React 18, which we still might do, but we need to fix 16 and 17 as well. Another in #3963 is to require a new strictMode prop to explicitly opt-into double incrementing on the server. That will be harder to do correctly, especially in libraries where StrictMode is unknown.

This PR implements an alternate approach using some React internals to get a stable reference to the Fiber node for the component. With this we can detect the second render and reset our counter back to what it was the first time, which in effect means it is only incremented once. This ensures the id never changes after the first render and therefore matches the server. See the code comment for a more detail explanation.

As mentioned before, we may still want to switch to React.useId for React 18 to handle other edge cases I'm not aware of with the new streaming renderer, though this should continue to work as before if you put an SSRProvider around every Suspense boundary. Switching that would change the format of the returned ids though, so might break people's unit tests (especially snapshots), so I am hesitant. Thoughts appreciated.

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. On the topic of whether to use React.useId for React 18, IMO it would be good to do this sooner rather than later. It feels like something that would be done eventually due to unknown edge cases (especially so we don't rely on the secret internals for too long) so breaking others tests is something we'll need to deal with eventually. IMO best to do that early while people may still be on earlier versions of React and thus won't have their test break as a result.

@rspbot
Copy link

rspbot commented Feb 2, 2023

@rspbot
Copy link

rspbot commented Feb 2, 2023

@rspbot
Copy link

rspbot commented Feb 2, 2023

## API Changes

unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }

@devongovett devongovett merged commit 97ff9f9 into main Feb 2, 2023
@devongovett devongovett deleted the useid-strict branch February 2, 2023 23:59
@LucasUnplugged
Copy link

@devongovett Not sure where's a better place for this comment, so posting here also, for visibility:
#3963 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants